fix(db): avoid detached datasource in SQL Server pool creator - #1383
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The lifecycle fix is covered by regression tests with no unresolved issues.
Review effort: Lite
Findings: None
What changed in this PR
Fixes SQL Server pool reconnect failures after a request-scoped datasource session closes.
Changes:
- Captures the datasource type as a plain value.
- Adds commit and rollback lifecycle regression tests.
| File | Summary |
|---|---|
backend/tests/test_sqlserver_pool_lifecycle.py |
Tests reconnect behavior after session closure. |
backend/apps/db/db.py |
Prevents detached ORM access during pool reconnection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1330.
A SQL Server connection pool created from a request-scoped
CoreDatasourceretained that ORM object in its connection creator. After the request session closed, a later pool recycle or reconnect could read the expiredds.typeattribute and raiseDetachedInstanceError. Capture the datasource type while building the engine so future connections use a plain value.The regression test creates the pool from a session-bound datasource, commits or rolls back and closes that session, then forces a reconnect from a later request. Both paths pass. The three existing pool manager tests also pass (5 total).
The same lifecycle was exercised with a reachable SQL Server instance (major version 16) through SQLBot's
exec_sql: the original code reproducedDetachedInstanceErrorafter forced recycle, while the fix returned the read-only query result. No database writes were performed. SQL Server 2019 and the full MaxKB-to-MCP workflow were not available for end-to-end verification.